-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: same origin spec bridges #23885
Conversation
Thanks for taking the time to open a PR!
|
Test summaryRun details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
c07c596
to
ee0da74
Compare
ee0da74
to
1fd4699
Compare
0a1086b
to
310d864
Compare
406d74d
to
d9c583b
Compare
62fc3ea
to
629cb7c
Compare
chore: refactor spec bridges to strictly enforce same origin fix: wrap fullCrossOrigin injection around feature flag inside buffered response
…it is sending unecessary cookies
@@ -296,17 +297,20 @@ const createApp = (port) => { | |||
}) | |||
|
|||
app.get('/test-request-credentials', (req, res) => { | |||
const origin = cors.getOrigin(req['headers']['referer']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since many of the requests are now same origin, we don't get the origin
header, so we grab it from the referrer for the cookie_behavior tests
@@ -207,7 +206,7 @@ const attachToWindow = (autWindow: Window) => { | |||
|
|||
cy.isStable(false, 'beforeunload') | |||
|
|||
cy.Cookies.setInitial() | |||
// NOTE: we intentionally do not set the cy.Cookies.setInitial() inside the spec bridge as we are not doing full injection and this leads to cookie side effects |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What side effects was this causing? Is this a separate issue from "same origin spec bridges" or was it exposed during these code changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Main issue is sometimes setting a cookie on the domain that isn't cleared in firefox that we don't even need because we don't do full injection. I would say it was always an issue and was exposed more so with these changes.
example within firefox before:
spec bridge was creating a __cypress.initial
cookie on foobar.com
domain (not .foobar.com
)
AUT is at www.foobar.com
and makes requests. __cypress.initial
cookie isn't attached because it doesn't fit the domain policy (foobar.com
vs .foobar.com
)
example within firefox after:
spec bridge was creating a __cypress.initial
cookie on www.foobar.com
domain (www.foobar.com
)
AUT is at www.foobar.com
and makes requests. __cypress.initial
cookie is attached and is never cleared
Not sure if this is an underlying issue with js-cookie
, but the domain that is set in firefox for the cookie is different than that of chromium browsers. We don't see it in chromium browsers because the domain set in the spec bridge is localhost
…s-io/cypress into proposal/same-origin-spec-bridges
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
User facing changelog
cy.origin
's url argument must be an exact origin match to the origin of the AUTAdditional details
In order to support google authentication within cy.origin and eliminate inconsistencies, we are moving away from same super domain origin spec bridge to strictly same origin spec bridges. This solves a few issues we have been running into with origin policy and introduces a breaking change into
Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation
? After looking at the documentation, all of our examples actually use same origin spec bridges 😄type definitions
?